[SPARK-19727][SQL][followup] Fix for round function that modifies original column#19576
[SPARK-19727][SQL][followup] Fix for round function that modifies original column#19576cloud-fan wants to merge 2 commits intoapache:masterfrom
Conversation
| precision: Int, | ||
| scale: Int, | ||
| roundMode: BigDecimal.RoundingMode.Value = ROUND_HALF_UP): Option[Decimal] = { | ||
| roundMode: BigDecimal.RoundingMode.Value = ROUND_HALF_UP): Decimal = { |
There was a problem hiding this comment.
Option is hard to use in java code(the codegen path), so I change the return type to nullable Decimal.
|
Test build #83059 has finished for PR 19576 at commit
|
| @@ -390,7 +390,7 @@ case class Cast(child: Expression, dataType: DataType, timeZoneId: Option[String | |||
| * | |||
There was a problem hiding this comment.
Nit: remove this useless line.
| ) | ||
| } | ||
|
|
||
| test("round/bround with table columns") { |
There was a problem hiding this comment.
This is an end-to-end test for code-gen path. Could we add a unit test case in MathExpressionsSuite?
There was a problem hiding this comment.
All the tests in MathExpressionsSuite are testing about literals, I think it's better to improve MathExpressionsSuite for attributes in a new PR, instead of doing it in a folllow-up PR. BTW the original PR didn't add test in MathExpressionsSuite either.
|
LGTM |
|
Test build #83176 has finished for PR 19576 at commit
|
|
Thanks! Merged to master/2.2 |
…ginal column ## What changes were proposed in this pull request? This is a followup of #17075 , to fix the bug in codegen path. ## How was this patch tested? new regression test Author: Wenchen Fan <wenchen@databricks.com> Closes #19576 from cloud-fan/bug. (cherry picked from commit 7fdacbc) Signed-off-by: gatorsmile <gatorsmile@gmail.com>
…ginal column ## What changes were proposed in this pull request? This is a followup of apache#17075 , to fix the bug in codegen path. ## How was this patch tested? new regression test Author: Wenchen Fan <wenchen@databricks.com> Closes apache#19576 from cloud-fan/bug. (cherry picked from commit 7fdacbc) Signed-off-by: gatorsmile <gatorsmile@gmail.com>
3 participants
What changes were proposed in this pull request?
This is a followup of #17075 , to fix the bug in codegen path.
How was this patch tested?
new regression test